fix: add consistent handling for all reasoning tag variants #8786
Conversation
- Updated presentAssistantMessage.ts to strip all reasoning tags (<think>, <thinking>, <reasoning>, <thought>) - Created ReasoningXmlMatcher utility to handle multiple reasoning tag variants - Updated all provider parsers to use ReasoningXmlMatcher - Ensures all four tag variants render identically as collapsible grey reasoning blocks Fixes #8785
dosubot
bot
added
the
size:L
Review SummaryI've reviewed the PR and identified the following issues that should be addressed: Issues to Address
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| private reasoningTags = ["think", "thinking", "reasoning", "thought"] | ||
| private results: Result[] = [] | ||
| private buffer = "" | ||
| private isProcessing = false |
There was a problem hiding this comment.
The private field isProcessing is declared but never used. Consider removing it if it's not needed.
| private isProcessing = false |
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| private extractMatchedResults(input: string): Result[] { | ||
| // Try each tag type to find matches | ||
| for (const tag of this.reasoningTags) { | ||
| // Check if the input contains this tag | ||
| if (input.includes(`<${tag}>`) || input.includes(`</${tag}>`)) { | ||
| const results = this.processWithTag(input, tag) | ||
| if (results.length > 0) { | ||
| // Transform results if needed | ||
| if (this.transform) { | ||
| return results.map(this.transform) | ||
| } | ||
| return results as Result[] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The extractMatchedResults method only processes one tag type per input. If a single buffer contains multiple different reasoning tag types (e.g., both <think> and <thinking>), only the first encountered tag type will be matched. Subsequent different tag types will be treated as unmatched text. This breaks the core requirement that all four tag variants should be handled uniformly. Consider using a strategy that can recognize all tag types simultaneously, such as a single regex pattern matching all variants or processing all tag types in a single pass.
| private reasoningTags = ["think", "thinking", "reasoning", "thought"] | ||
| private results: Result[] = [] | ||
| private buffer = "" | ||
| private isProcessing = false |
There was a problem hiding this comment.
The isProcessing field is declared but never used. Consider removing it to keep the code clean.
2 participants
Summary
This PR addresses Issue #8785 by ensuring all four reasoning tag variants (
<think>,<thinking>,<reasoning>,<thought>) are treated consistently and rendered as the same collapsible grey reasoning block.Problem
Previously, Roo Code processed reasoning tags inconsistently:
<think>rendered correctly as a collapsible grey reasoning block<thinking>had its tags stripped but text remained visible<reasoning>and<thought>were not recognized at allSolution
presentAssistantMessage.tsto handle all four reasoning tag variants uniformlyChanges
src/core/assistant-message/presentAssistantMessage.tsto strip all reasoning tag variantssrc/utils/reasoning-xml-matcher.tsutility classTesting
Fixes #8785
Important
This PR ensures consistent handling of reasoning tags by introducing
ReasoningXmlMatcherand updating relevant files to use it.<think>,<thinking>,<reasoning>,<thought>tags inpresentAssistantMessage.ts.ReasoningXmlMatcherto match multiple reasoning tag variants.cerebras.ts,chutes.ts,featherless.tsto useReasoningXmlMatcher.ReasoningXmlMatcherinreasoning-xml-matcher.spec.ts.This description was created by
for 620c3cd. You can customize this summary. It will automatically update as commits are pushed.